Skip to content

test(assets-controller): add stale-balance scenario integration tests for accounts API v6 - #10584

Merged
Prithpal-Sooriya merged 14 commits into
mainfrom
test/assets-controller-stale-balance-scenarios
Sep 30, 2026
Merged

Prithpal-Sooriya merged 14 commits into
mainfrom
test/assets-controller-stale-balance-scenarios

Conversation

@Prithpal-Sooriya

@Prithpal-Sooriya Prithpal-Sooriya commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

Adds AssetsController.stale-balance-scenarios.integration.test.ts, an AssetsController-level integration suite covering all 12 stale-balance scenarios from the accounts API v6 integration decision doc (full and merge tables).

The suite boots the real AssetsController (with assetsAccountsApiV6: true) and mocks only the external boundaries, following the pattern of AssetsController.bsc-spam-token-filtering.integration.test.ts:

  • HTTP APIs (Accounts API v6 + v2-supportedNetworks, tokens /v3/assets, prices /v3/spot-prices) are nocked from real captured responses in src/__fixtures__/stale-balance/api-responses/. Every fixture mutation is documented inline (balance doubling to model wallet movement between passes, occurrences stamping, chain omissions for RPC fallback).
  • RPC / snap / lifecycle are exercised through mock messenger action handlers (NetworkController, SnapController, KeyringController, staking reads) and live events (AccountActivityService:balanceUpdated, AccountsController:accountBalancesUpdated, TransactionController:transactionConfirmed, KeyringController:unlock).

Scenario coverage (7 full + 5 merge): snap failure carry-over, skipped-asset last-amount copy, zero-seeding for never-seen visible assets, RPC fallback carry-over on failing balanceOf reads, disappearing zero-balance seeds, failed-fetch carry-over, fresh amounts on healthy fallback, event overlays, malformed-row handling, wrong-amount correction at the next full poll, snap-event overlay + refresh, and staked ETH on mainnet + Hoodi across an unlock lifecycle with a staking deposit.

Test plan

  • yarn workspace @metamask/assets-controller run test
    • New suite: 12/12 scenarios pass
    • Full package suite passes
  • yarn lint checks on the new files: oxlint and oxfmt clean
  • yarn build (root, full monorepo type-check) passes

Notes for reviewers

  • Fixture balance-row shape includes unit, required by the keyring API's Balance struct via strictMask (@metamask/keyring-api); the controller discards it downstream.
  • Native ETH on Hoodi is keyed under the zero-address ERC-20 encoding (eip155:560048/erc20:0x0000000000000000000000000000000000000000) — the controller's documented fallback for EVM chains absent from its native-asset seed map, which excludes testnets.
  • The Multicall3 aggregate3 fixture encodes the contract's real return shape — Result[] only, with no leading blockNumber (only aggregate/tryAggregate return one). An earlier draft of the fixture encoded (uint256, Result[]), which made every batch decode throw and silently fall through to single balanceOf calls; caught in review and fixed, so the scenarios exercise the batched read path. MulticallClient's decoder is correct as shipped — no product bug here.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed — no entries needed: test-only, non-consumer-facing changes (no-changelog label applied)
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — N/A, no breaking changes

Note

Low Risk
Test-only additions with mocked boundaries; no changes to runtime balance merge or fetch behavior shipped to consumers.

Overview
Adds AssetsController.stale-balance-scenarios.integration.test.ts, an integration suite that runs the real AssetsController with assetsAccountsApiV6: true and exercises all 12 stale-balance scenarios from the accounts API v6 decision doc (Solana snap failures/skips, BNB Chain RPC fallback when v6 marks a chain unprocessed, Account Activity live overlays/malformed rows, and staked ETH across mainnet + Hoodi after unlock and a staking tx).

Supporting changes introduce a __fixtures__/stale-balance/ tree: real HTTP captures for Accounts/Tokens/Prices APIs wired through mockStaleBalanceApis, mutable mock EVM RPC (Multicall3 aggregate3, failing tokens, staking reads), Solana keyring snap handlers, wallet/account builders, and messenger registration for BSC/mainnet/Hoodi networks. getIgnoringCase is added to test-utils for case-insensitive balance assertions, and tsc-suppressions.json picks up suppressions for fixture/test typing.

There is no production logic change in this PR—only tests, fixtures, and type suppressions.

Reviewed by Cursor Bugbot for commit 245e36c. Bugbot is set up for automated code reviews on this repo. Configure here.

Prithpal-Sooriya and others added 9 commits September 28, 2026 15:29
… for accounts API v6

Cover all 12 stale-balance scenarios documented in the accounts API v6
integration decision doc (7 `full` + 5 `merge`), booting the real
AssetsController with mocked external boundaries: nock for the Accounts /
tokens / prices HTTP APIs, and mock messenger action handlers for RPC,
snap, and lifecycle events. Fixtures build on real API captures, with
every fixture mutation documented inline.
…-balance suite

Follow the shared-utils schema of the other AssetsController integration
suites: `getIgnoringCase` moves to `__fixtures__/test-utils.ts` instead of
a per-suite copy, and the account builders in
`__fixtures__/stale-balance/staleBalanceWallet.ts` delegate to the shared
`createMockInternalAccount` (as `AssetsController.spam-cleanup.integration
test.ts` does) rather than hand-rolling `InternalAccount` boilerplate.
…teSurface assertion utils

- The Multicall3 `aggregate3` fixture no longer prefixes a `blockNumber`
  `uint256` before the results array: the real contract returns `Result[]`
  only (only `aggregate`/`tryAggregate` return a block number), so the
  batched read path now decodes instead of silently falling back to
  single `balanceOf` calls (review finding).
- Amount assertions now go through `StateSurface`-style utils
  (`buildBalancesSurface` + per-account `SOLANA_BALANCES`/`BSC_BALANCES`/
  `MAINNET_BALANCES`), matching the other AssetsController integration
  suites.
…cenario describe

Adopt the exact `StateSurface` pattern of the bsc-spam integration
suite: a bare `type StateSurface` and a `const BALANCES: StateSurface`
whose lookUp binds the account's balances record — copied verbatim into
each scenario describe (the wallet spans three accounts, so each
describe binds its own), replacing the `buildBalancesSurface` factory.
…fix, and verbatim StateSurface utils

The three commits below (d3c9d4f, 48b3abd, 1c100ff) landed
empty — their changes were never staged before the plumbing commit, so
only the SHAs moved. This commit actually lands their changes:

- Reuse the shared integration-test utils: consolidate `getIgnoringCase`
  into `__fixtures__/test-utils.ts` and delegate the account builders in
  `staleBalanceWallet.ts` to the shared `createMockInternalAccount`.
- Fix the Multicall3 `aggregate3` fixture's wire shape: drop the
  `blockNumber` `uint256` prefix — the real contract returns `Result[]`
  only, so the batched read path decodes instead of silently falling
  back to single `balanceOf` calls.
- Copy the bsc-spam suite's `StateSurface` utils verbatim: bare
  `type StateSurface` and a `const BALANCES: StateSurface` per scenario
  describe (this wallet spans three accounts), with all amount
  assertions reading `surface.lookUp`.
Assert with the bsc-spam-token exemplar dialect (direct
BALANCES.lookUp(...).toMatchObject, inline key-set checks) instead of the
invented expectAmount/expectedAmount/lowerKeySet helpers; carry-over states
inline via buildEmptyStaleBalanceState. Drop all banner separators and
rationale comments from the suite and the __fixtures__/stale-balance helpers,
keeping one-to-two-line summaries only; remove the dead
USDT_ASSET_ID_LOWERCASE constant.
…e-this-sp0

# Conflicts:
#	packages/assets-controller/src/__fixtures__/test-utils.ts
Per review: add getAssetIds and lookupAmount to each describe's BALANCES
surface so the repeated lower-cased key-set checks and amount assertions
read expect(BALANCES.getAssetIds(state)).toStrictEqual(...) and
expect(BALANCES.lookupAmount(state, assetId)).toBe(...) across every
scenario. Drop the scenario-3 key-set check (redundant with its balance
assertions) and replace scenario-11's with a direct USDC amount
assertion; S5/S8/S9/S10 key-sets now compare surfaces of captured states.
@Prithpal-Sooriya
Prithpal-Sooriya marked this pull request as ready for review September 30, 2026 08:40
@Prithpal-Sooriya
Prithpal-Sooriya requested a review from a team as a code owner September 30, 2026 08:40
@Kriys94

Kriys94 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Mainly looked packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts and that looks good to me 👍

Kriys94
Kriys94 previously approved these changes Sep 30, 2026
@Kriys94
Kriys94 added this pull request to the merge queue Sep 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 30, 2026
@Prithpal-Sooriya

Copy link
Copy Markdown
Contributor Author

Mainly looked packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts and that looks good to me 👍

Thanks @Kriys94 - yep this was the main file that needed review. Good call on realistic data capture - I'm re-executing this now.

…ted API captures

Re-executed every capture query behind the stale-balance suite against the
live APIs and rebuilt the fixture files from the 2026-09-30 responses, so
each fixture is a verbatim capture whose header links the exact query made,
including the asset ID lists for the v3 spot-price and asset queries.

The accounts /v2/supportedNetworks capture now keeps solana in
partialSupport as the live API returns it; the keyring snap still owns the
Solana account in the suite (all 12 scenarios pass). Spot prices and a few
drifted balances refresh to the new capture; USDT/GTAI/BNB/ETH rows are
byte-identical to the previous capture.
Prithpal-Sooriya and others added 2 commits September 30, 2026 15:15
Main added a lint:tsc:check job that compares repo-wide tsc errors
against tsc-suppressions.json. The stale-balance suite files carry
pre-existing latent test-file type errors that no CI job previously
checked, so the new gate fails without suppressions. Merge main to pick
up the tooling and record the 14 errors across the 5 suite files with
the repo's own lint:tsc:check --suppress-all script.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8bef618. Configure here.

@Prithpal-Sooriya

Copy link
Copy Markdown
Contributor Author

While re-capturing the fixtures I also fixed the newly-failing Lint (lint:tsc:check) job (heads d028245 and earlier):

  • lint:tsc:check was added on main after this branch's last sync. It runs repo-wide tsc --build tsconfig.lint.json and compares every error against the root tsc-suppressions.json.
  • This suite's fixture and test files carry 14 latent test-file type errors that no previous CI job checked (yarn build excludes test files, and Jest does not type-check), so the new gate flagged them as unsuppressed.
  • None of them are caused by the fixture rebuild: 9 sit in the suite's own harness code (messenger.ts, provider.ts, staleBalanceWallet.ts, the integration test) that this change never touched; the 5 in the nock-wiring index.ts are the same pre-existing pattern (loose stub values vs as const fixture literal types), only now spelled with the new capture's literals.

Fix (8bef618): synced main to pick up the tooling and suppressions file, then recorded the 14 errors with the repo's own yarn lint:tsc:check --suppress-all — no source or test-behavior change. Verified locally: lint:tsc:check passes, stale-balance suite 12/12, full assets-controller package 1286/1286, lint:misc:check clean. All 346 checks green on 8bef618.

Prithpal-Sooriya and others added 2 commits September 30, 2026 15:38
…f suppressing

Rather than adding the suite's 14 latent test-file type errors to
tsc-suppressions.json, fix them in place:

- Give V6BalanceEntry a structural type so the bsc-derived literal type
  stops rejecting mainnet rows, balance-replacement spreads, and the
  zero-balance stub.
- Annotate mockStaleBalanceApis's options with the shared
  StaleBalanceApiMutations type; they were inferred as never[]/{}.
- Complete the account metadata in the wallet builders with keyring
  and importTime defaults, mirroring createMockInternalAccount.
- Complete the transactionConfirmed payload's TransactionMeta fields
  (id, networkClientId, status, time).
- Cast defaultAbiCoder.encode's result to Hex; it always returns hex.

Keep @ts-expect-error only for the deliberately malformed balance
rows and Hoodi's zero-address ERC-20 native-asset fallback, which the
extension itself uses for chains missing from the slip44 map.

Drop the tsc-suppressions.json entries added in 8bef618.
@Prithpal-Sooriya

Copy link
Copy Markdown
Contributor Author

Follow-up on the lint:tsc:check fix: 8bef618 had recorded the 14 latent type errors in tsc-suppressions.json. Reconsidering — the suppressions file is a ratchet for pre-existing debt, so a brand-new suite shouldn't add to it. 3ab8cb0 fixes the errors in place instead and drops those suppression entries (the file is now identical to main):

  • V6BalanceEntry now has a structural type instead of being inferred from the BSC capture's literals (which is what rejected the mainnet rows, the balance-replacement spreads, and the zero-balance stub).
  • mockStaleBalanceApis options are now annotated StaleBalanceApiMutations (shared with the v6 mock; they previously inferred never[]/{} defaults).
  • The wallet builders complete account metadata with keyring/importTime defaults, mirroring createMockInternalAccount.
  • The transactionConfirmed payload completes its TransactionMeta fields (id, networkClientId, status, time) — the listener only reads chainId/txParams, so behavior is unchanged.
  • defaultAbiCoder.encode's result is cast to Hex (it always returns hex, but is typed string).

@ts-expect-error remains in exactly two intentional places: the malformed-row scenario (the rows deliberately violate BalanceUpdate) and Hoodi's native-asset identifier, which uses the zero-address ERC-20 fallback the extension itself applies to chains missing from the slip44 map (see NATIVE_ASSETS).

Verified: lint:tsc:check clean with no new suppressions, suite 12/12, full assets-controller package 1286/1286, lint:misc:check + lint:oxlint clean, and all checks green on 3ab8cb0.

@Prithpal-Sooriya
Prithpal-Sooriya added this pull request to the merge queue Sep 30, 2026
Merged via the queue into main with commit f4927a3 Sep 30, 2026
53 checks passed
@Prithpal-Sooriya
Prithpal-Sooriya deleted the test/assets-controller-stale-balance-scenarios branch September 30, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants